Easy metrics based on replicate and precursor annotation values#1210
Easy metrics based on replicate and precursor annotation values#1210ankurjuneja wants to merge 17 commits into
Conversation
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
A few minor suggestions and a larger request to make one of the other metric dialogs consistent with this new approach.
| }); | ||
| jQuery('#createNewTraceMetricButton').click(function() { | ||
| LABKEY.internal.ConfigureQCMetrics.addNewMetric('trace') | ||
| LABKEY.internal.ConfigureQCMetrics.addNewMetric('trace'); |
There was a problem hiding this comment.
Use a constant here and other places, including for annotation
| else if (configuration.getAnnotationName() != null) | ||
| { | ||
| // annotation-backed metrics: escape the annotation name for SQL string literal | ||
| String escapedName = configuration.getAnnotationName().replace("'", "''"); |
There was a problem hiding this comment.
Let's use the metric name instead of the annotation name.
There was a problem hiding this comment.
using metricName as label
| function getAnnotationTarget() { | ||
| return $('input[name="annotationType"]:checked').val() === 'precursor' | ||
| ? 'precursor_result' | ||
| : 'replicate'; |
There was a problem hiding this comment.
In the custom query metric dialog and in the table of metrics we use "run" instead of "replicate". I think "replicate" is better. Can you update the other dialog and table to be consistent with the Precursor and Replicate?
| const isPrecursor = op === 'update' && metric.PrecursorScoped; | ||
| const title = op === 'insert' ? 'Add Annotation-Backed Metric' : 'Edit Annotation-Backed Metric'; | ||
|
|
||
| return '<div id="' + DIALOG_ID + '" style="position:fixed;top:0;left:0;width:100%;height:100%;background:rgba(0,0,0,0.5);z-index:9999;display:flex;align-items:center;justify-content:center;">' |
There was a problem hiding this comment.
This is somewhat frustratingly inconsistent with the query based custom metric dialog, which has almost identical controls. The fields are in different orders, we use a radio vs a drop-down for replicate vs precursor, and the button layout is different.
While I'd like to refactor all of this into React soon, can you adapt the query metric dialog to use this plain HTML/JS approach too?
…8.sql Co-authored-by: Josh Eckels <jeckels@labkey.com>
…dow.js Co-authored-by: Josh Eckels <jeckels@labkey.com>
2 participants
Rationale
https://github.com/LabKey/internal-issues/issues/1046
Related Pull Requests
Changes